feat(rest): implement OAuth2 token auto-refresh for REST catalog#646
feat(rest): implement OAuth2 token auto-refresh for REST catalog#646lishuxu wants to merge 9 commits into
Conversation
| {std::string(kAuthorizationHeader), std::string(kBearerPrefix) + token_}}; | ||
|
|
||
| // Update expiration | ||
| if (response.expires_in_secs.has_value()) { |
There was a problem hiding this comment.
Reset expires_at_ before deriving the new expiry, and skip scheduling when the refreshed token has neither expires_in nor JWT exp.
There was a problem hiding this comment.
Fixed. Now resetting expires_at_ to epoch before computing the new value.
| .client_id = client_id, | ||
| .client_secret = client_secret, | ||
| .scope = scope, | ||
| .keep_refreshed = true, |
There was a problem hiding this comment.
Pass the parsed token-refresh-enabled value into this config instead of forcing refresh on. Users who disable refresh should not get background token requests.
There was a problem hiding this comment.
Fixed. Added keep_refreshed parameter to MakeOAuth2, passed from config.keep_refreshed().
Replace the MakeOAuth2 stub with a full OAuth2AuthSession that
automatically refreshes tokens before expiration using the
client_credentials grant.
Key components:
- OAuth2AuthSession: manages token lifecycle with shared_mutex for
concurrent read access and background refresh via scheduler
- TokenRefreshScheduler: process-global singleton with a single worker
thread that fires delayed refresh callbacks
- ExpiresAtMillis: JWT exp claim parser for determining token expiry
when expires_in is not provided in the token response
- Base64Decode/Base64UrlDecode added to TransformUtil as public utilities
Replace the MakeOAuth2 stub with a full OAuth2AuthSession that automatically refreshes tokens before expiration using the client_credentials grant. Key components: - OAuth2AuthSession: manages token lifecycle with shared_mutex for concurrent read access and background refresh via scheduler - TokenRefreshScheduler: process-global singleton with a single worker thread that fires delayed refresh callbacks - ExpiresAtMillis: JWT exp claim parser for determining token expiry when expires_in is not provided in the token response - Base64Decode/Base64UrlDecode added to TransformUtil as public utilities
Replace the MakeOAuth2 stub with a full OAuth2AuthSession that automatically refreshes tokens before expiration using the client_credentials grant.
Replace the MakeOAuth2 stub with a full OAuth2AuthSession that automatically refreshes tokens before expiration using the client_credentials grant.
Replace the MakeOAuth2 stub with a full OAuth2AuthSession that automatically refreshes tokens before expiration using the client_credentials grant.
Replace the MakeOAuth2 stub with a full OAuth2AuthSession that automatically refreshes tokens before expiration using the client_credentials grant.
Replace the MakeOAuth2 stub with a full OAuth2AuthSession that automatically refreshes tokens before expiration using the client_credentials grant.
Replace the MakeOAuth2 stub with a full OAuth2AuthSession that automatically refreshes tokens before expiration using the client_credentials grant.
| }; | ||
|
|
||
| /// \brief Create an OAuth2 session and optionally schedule refresh. | ||
| static std::shared_ptr<OAuth2AuthSession> Create( |
There was a problem hiding this comment.
| static std::shared_ptr<OAuth2AuthSession> Create( | |
| static std::shared_ptr<OAuth2AuthSession> Make( |
nit: rename it to Make to be consistent with this repo.
| /// (a single HTTP POST to refresh a token), so one thread is sufficient. | ||
| /// | ||
| /// Thread safety: All public methods are thread-safe. | ||
| class ICEBERG_REST_EXPORT TokenRefreshScheduler { |
There was a problem hiding this comment.
@HuaHuaY is working on the adding a thread pool abstraction and it would be good to reuse that once available. The main issue of current design is that only a single thread is working on token refresh and a slow request would starve all other tasks.
| props.mutable_configs().insert_or_assign(key, value); | ||
| } | ||
|
|
||
| auto result = FetchToken(client_, *empty_session, props); |
There was a problem hiding this comment.
Java refreshes through token exchange by default and only falls back to client credentials in specific cases. This path always uses client_credentials and ignores token-exchange-enabled, which breaks parity for exchange-based refresh.
| config.optional_oauth_params(), client); | ||
| } | ||
|
|
||
| // If token is provided, use it directly. |
There was a problem hiding this comment.
Java wraps configured access tokens in fromAccessToken() so expiring tokens can be refreshed. Returning a static session here means JWT or token-expires-in-ms based tokens never refresh.
| /// | ||
| /// Handles optional padding ('='). | ||
| /// \return Decoded string, or an error if the input contains invalid characters. | ||
| static Result<std::string> Base64Decode(std::string_view encoded); |
There was a problem hiding this comment.
nit: we can refactor a separate iceberg/util/base64.h for these Base64XXX functions.
| /// Handles optional padding ('='). This variant uses '-' and '_' instead of | ||
| /// '+' and '/' per RFC 4648 §5. | ||
| /// \return Decoded string, or an error if the input contains invalid characters. | ||
| static Result<std::string> Base64UrlDecode(std::string_view encoded); |
There was a problem hiding this comment.
Do we need to add a Base64UrlEncode for parity?
3 participants
Replace the MakeOAuth2 stub with a full OAuth2AuthSession that automatically refreshes tokens before expiration using the client_credentials grant.